-
-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide musl libc compatibility #696
Conversation
With musl and POSIX, they're signed. With glibc, they're unsigned.
Codecov Report
@@ Coverage Diff @@
## main #696 +/- ##
=======================================
Coverage 82.93% 82.94%
=======================================
Files 62 62
Lines 15926 15950 +24
=======================================
+ Hits 13209 13229 +20
- Misses 2717 2721 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@folkertdev Maybe you want to revert the merge pending a review on the safety aspect.
timespec.tv_sec += offset_secs as libc::time_t; | ||
timespec.tv_sec += { | ||
// this looks a little strange. it is to work around the `libc::time_t` type being | ||
// deprectated for musl in rust's libc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to explicitly silence this warning instead. There's no recommended migration path. It's questionable whether the deprecation will be upheld. See: rust-lang/libc#1848
(Typo: deprecated.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I read the issue, and ... it's really unclear what they actually want to happen there. it must eventually be changed because that is just what musl expects now, right? I'll amend this to ignore the warning, and link to the issue
Musl libc is a popular alternative to glibc. It was designed for simplicity, correctness and efficiency. It has some slight incompatibilities with glibc.
Musl libc is used by e.g., Alpine Linux and Gentoo as overlay. Alpine Linux is a popular OS for container images.
This change also allows for compilation to statically linked binaries.